Skip to content

feat(config): add review.max_findings caps for blockers/nits (#2049) - #3759

Closed
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:feat/review-max-findings-2049
Closed

feat(config): add review.max_findings caps for blockers/nits (#2049)#3759
bohdansolovie wants to merge 1 commit into
JSONbored:mainfrom
bohdansolovie:feat/review-max-findings-2049

Conversation

@bohdansolovie

Copy link
Copy Markdown
Contributor

Summary

  • Add review.max_findings: { blockers?, nits? } to the focus manifest — non-negative integer display caps parsed with warn+ignore on invalid values
  • Truncate rendered blockers/nits in the unified comment with a deterministic +N more footer; gate decisions are unchanged (display-only)
  • When the whole object is absent, keep the legacy 12-item cap per list (byte-identical)

Fixes #2049

Test plan

  • focus-manifest.test.ts — parse, round-trip, invalid warns, resolveReviewPromptOverrides
  • max-findings-display.test.ts — truncate helper, renderer caps, gate-invariant, bridge wiring
  • config-templates.test.ts — documents max_findings in example yml files
  • npx tsc -p tsconfig.json --noEmit passes locally
  • CI green (unit + codecov patch ≥99%)

Made with Cursor

Configurable per-repo limits on how many blockers/nits render in the unified
comment, with a +N more footer. Display-only — gate decisions unchanged.
Absent config keeps the legacy 12-item cap (byte-identical).

Fixes JSONbored#2049

Co-authored-by: Cursor <cursoragent@cursor.com>
@bohdansolovie
bohdansolovie requested a review from JSONbored as a code owner July 6, 2026 07:44
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
10030 1 10029 7
View the top 1 failed test(s) by shortest run time
test/unit/max-findings-display.test.ts > renderUnifiedReviewComment max_findings display caps (#2049) > keeps the legacy 12-nit cap when maxFindings is omitted (byte-identical)
Stack Traces | 0.0399s run time
AssertionError: expected '> [!TIP]\n> 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩\…' not to contain 'more nit(s) not shown'

- Expected
+ Received

- more nit(s) not shown
+ > [!TIP]
+ > 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩
+ >
+ > ### ✅ Gittensory review result - approve/merge recommended
+ >
+ > `2 files` · `1 AI reviewer` · `no blockers`
+ >
+ > **✅ Suggested Action - Approve/Merge**
+ > - safe to merge
+ >
+ > **Review summary**
+ > Looks good.
+ >
+ > <details><summary><b>Nits</b> — 12 non-blocking (+1 more)</summary>
+ >
+ > - [ ] Distinct nit 1
+ > - [ ] Distinct nit 2
+ > - [ ] Distinct nit 3
+ > - [ ] Distinct nit 4
+ > - [ ] Distinct nit 5
+ > - [ ] Distinct nit 6
+ > - [ ] Distinct nit 7
+ > - [ ] Distinct nit 8
+ > - [ ] Distinct nit 9
+ > - [ ] Distinct nit 10
+ > - [ ] Distinct nit 11
+ > - [ ] Distinct nit 12
+ >
+ > _+1 more nit(s) not shown._
+ > </details>
+ >
+ > | Signal | Result | Evidence |
+ > |---|---|---|
+ > | **Code review** | ✅ No blockers | 1 reviewer |
+ >
+ > <sub>🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed</sub>

 ❯ test/unit/max-findings-display.test.ts:59:20

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 6, 2026
@loopover-orb

loopover-orb Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-06 07:55:12 UTC

10 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI failing · blocked

🛑 Suggested Action - Reject/Close

  • Linked issue overlaps another open PR — Review the related PRs before spending reviewer time on duplicate work.

Review summary
Adds `review.max_findings` display-only caps for blockers/nits in the unified comment: focus-manifest parsing (non-negative int per field, warn+ignore on invalid, byte-identical when absent via new EMPTY_MAX_FINDINGS_CONFIG), a pure `truncateDisplayedFindingLines` helper applied after the existing dedupe step, and wiring through resolveReviewPromptOverrides → processors.ts → unified-comment-bridge → renderUnifiedReviewComment. The legacy 12-item cap is preserved by treating `ctx.maxFindings === undefined` as LEGACY_FINDINGS_DISPLAY_CAP, and gate/blocker semantics are explicitly untouched (only rendered lists are truncated), which the test suite verifies directly (byte-identical legacy path, per-field null=uncapped, gate-invariant test). Test coverage is thorough (parse/round-trip/invalid warnings, truncate helper, renderer caps, bridge wiring, config-template docs).

Nits — 5 non-blocking
  • src/review/unified-comment.ts: `dedupeLines`'s `cap` param lost its implicit default of 12 (now `cap?: number` with no default, since capping is applied later via `truncateDisplayedFindingLines`) — since the full file isn't visible here, confirm no other caller of `dedupeLines`/`bullets` in this file relied on the old implicit 12-item truncation, or it now silently renders unbounded lists.
  • src/queue/processors.ts: `maxFindingsForReview` is only spread into the unified-comment branch's args (the `if` block around line 9502) — confirm the `else` branch using `buildPublicPrIntelligenceComment(commentArgs)` intentionally doesn't need the cap rather than silently dropping the feature on that code path.
  • A cap of `0` still renders an empty 'Nits'/blockers section header with just a '+N more' footer (src/review/unified-comment.ts renderUnifiedReviewComment) — worth a one-line note on whether that's the intended UX for `max_findings: { nits: 0 }`.
  • Add an explicit test asserting `buildPublicPrIntelligenceComment`'s behavior (or its exclusion) when `max_findings` is configured, to make the branch coverage in processors.ts explicit rather than implicit from the diff.
  • Consider a short code comment at the `dedupeLines` cap-param call sites noting that truncation now happens downstream via `truncateDisplayedFindingLines`, to prevent a future edit from reintroducing double-capping.

Why this is blocked

  • Linked issue overlaps another open PR — Review the related PRs before spending reviewer time on duplicate work.

CI checks failing

  • validate
  • validate-code
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #2049
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 354 registered-repo PR(s), 208 merged, 7 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bohdansolovie; Gittensor profile; 354 PR(s), 7 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: bohdansolovie
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, C++, JavaScript
  • Official Gittensor activity: 354 PR(s), 7 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb

loopover-orb Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-code); Linked issue overlaps another open PR; duplicate of another open PR). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(config): add review.max_findings caps for blockers/nits

1 participant